feat(codex): initialize account picker selectors - #1152
Conversation
📝 WalkthroughWalkthroughCodex account selector validation now reserves the ChangesNamespace validation and allocation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Config as OcxConfig
participant Validator as Selector validation
participant ProfileEditor as Routing-profile editor
participant Storage as Persistence and catalog
Config->>Validator: validate policy and routing-profile namespace collisions
ProfileEditor->>Validator: validate profile alias
Validator-->>ProfileEditor: accept or return invalid_profile
ProfileEditor->>Storage: persist and refresh only after validation
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Looks good from my side overall. I didn’t find a code-level blocker. Before I approve, please:
If the rebased diff stays clean and CI passes, I’m good to approve. |
813bd7f to
e73ed2b
Compare
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Rebased and pushed. The current head is I also added JSDoc for the changed namespace helper and the new public policy namespace constant to clear the 66.67% coverage warning. Local checks on this head are green: all four isolated Linux shards (9,636 pass / 11 skip), GUI tests (646 pass), focused selector tests (170 pass), typecheck, privacy scan, GUI build, release-script build, CLI smoke, hygiene, and sponsorship checks. The PR is back Ready with the 4/4 checklist complete. GitHub's fork workflows still need maintainer approval before they can run: CodeRabbit is also re-reviewing the final head. |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/codex/account-namespaces.ts (1)
76-90: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winCheck runtime consumers for routing-profile defaults.
src/codex/account-namespaces.tsdefinesinitializeDefaultCodexAccountNamespacesandappendDefaultCodexAccountNamespacein lines 115 and 139, and no tracked runtime call site currently passes them. Add the callers through the shared config flow or require the sameroutingProfilesshape there; otherwise generated selectors can still collide with routing-profile alias prefixes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/codex/account-namespaces.ts` around lines 76 - 90, Update the shared configuration flow around initializeDefaultCodexAccountNamespaces and appendDefaultCodexAccountNamespace so runtime callers provide routingProfiles, or route initialization through the existing config path that does. Ensure generated account selectors reserve routing-profile alias prefixes consistently with occupiedNamespaces and defaultCodexAccountNamespaces, preventing collisions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/codex/account-namespaces.ts`:
- Around line 76-90: Update the shared configuration flow around
initializeDefaultCodexAccountNamespaces and appendDefaultCodexAccountNamespace
so runtime callers provide routingProfiles, or route initialization through the
existing config path that does. Ensure generated account selectors reserve
routing-profile alias prefixes consistently with occupiedNamespaces and
defaultCodexAccountNamespaces, preventing collisions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 02f31d98-a569-46ce-bbb3-f5cabd50e1a8
📒 Files selected for processing (2)
src/codex/account-namespaces.tssrc/routing/profile-namespace.ts
|
Checked the outside-diff CodeRabbit note. No code change is needed in this slice: both selector helpers already require |
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
One upstream CI job needs a maintainer rerun: |
|
@Wibias, the fork run you approved completed with the runner-only Bun failure described just above. Could you rerun the failed jobs when you have a moment? GitHub blocks contributor reruns; the exact shard is green locally on this head. |
|
@Wibias, thank you—the failed-job rerun passed. All required checks are now green, GitHub reports the PR clean and mergeable, and there are no unresolved review threads. |
2 participants
Summary
policynamespace and existing slash-qualified routing-profile prefixes during default and append allocationThis is the second slice requested in #1019, following #1096. It intentionally contains only selector initialization and collision foundations. Catalog convergence and management API/dashboard activation remain in later slices.
Part of #425.
Verification
dev@44dce33abun run typecheckbun run privacy:scanbun test tests/repo-hygiene.test.tsnode --test .github/scripts/pr-sponsored-surface.test.cjsgit diff --checkbun test --isolate tests --shard=N/4): 9,636 passed, 11 skipped, 0 failedReview readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
New Features
Bug Fixes
policyand case-insensitive variants.Documentation